Fix inconsistencies in feature selector processing part 2: pseudo block instances#78326
Conversation
|
Flaky tests detected in cf5a4c1. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/26205136685
|
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +349 B (0%) Total Size: 7.98 MB 📦 View Changed
ℹ️ View Unchanged
|
922778c to
5c15d57
Compare
talldan
left a comment
There was a problem hiding this comment.
In terms of testing it behaves much better than trunk.
Some potential code issues were spotted (mostly by codex!)
I think at some point we (WordPress / Gutenberg) might need to invest in better parsing for selectors. Probably into an AST or something, it'd result in much more robust handling vs. regex and other text manipulation strategies.
| return $base_selector . $state; | ||
| } | ||
|
|
||
| $selectors = explode( ',', $block_selector ); |
There was a problem hiding this comment.
I think there's a similar issue here to the other PR, it'll split on something like :not(.foo, .bar).
Not sure if it's possible to borrow what you did in the other PR or alternatively use the same fix.
There was a problem hiding this comment.
The other PR added a protected function to the theme.json class, so that won't be reusable here. We can add a similar function here i guess.
| const selectors = blockSelector | ||
| .split( ',' ) | ||
| .filter( ( selector ) => selector.trim() ); |
There was a problem hiding this comment.
Similar to the other comment, this would split :not(.foo, .bar).
| if ( preg_match( '/^[^ >+~]+[ >+~](.*)$/', $selector, $matches ) ) { | ||
| $scoped_selectors[] = $base_selector . ' ' . trim( $matches[1] ) . $state; | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Codex spotted two potential issues around this code (and similar code in JS)
I'm happy to have it try to push fixes if you like, given it has the context.
Modifier classes get dropped from feature selectors
Example:
.wp-block-search.wp-block-search__button-outside .wp-block-search__input
becomes effectively:
.wp-states-x .wp-block-search__input:hover
Risk: conditions encoded in the first compound selector are lost. If pseudo states expand beyond Button/Nav or custom blocks use this shape, styles can apply too broadly.
Child combinator formatting changes selector meaning
Example:
.wp-block-foo>.inner
can become:
.wp-states-x .inner:hover
instead of:
.wp-states-x > .inner:hover
Risk: valid selector formatting without spaces changes behavior.
There was a problem hiding this comment.
Example: .wp-block-search.wp-block-search__button-outside .wp-block-search__input
becomes effectively: .wp-states-x .wp-block-search__input:hover
oooh is that why the search block was looking unusually interesting in my testing? 😄
yeah if you have a fix locally feel free to push it!
5c15d57 to
38a0003
Compare
|
@tellthemachines I've pushed some fixes for those issues, let me know if that looks ok. |
| * Preserve anything after that prefix, including modifier classes on the | ||
| * same element and combinators without spaces. | ||
| */ | ||
| if ( preg_match( '/^([.#]?[-_a-zA-Z0-9]+|\[[^\]]+\])/', $selector, $matches ) ) { |
There was a problem hiding this comment.
I'm not sure that we have explicit rules around what's allowed as a block selector, but we shouldn't allow ids because there's no way of preventing more than one of that block being added to a page. However, this is probably not the right place to enforce that.
Yeah those changes look OK! I'm wondering if it would be better to put the |
38a0003 to
7e4a94c
Compare
What?
Complementary to #78276; follow-up from #76491.
Addresses feature selector creation for styles applied to block instance pseudo states.
In trunk, width and text orientation styles applied to pseudo states of the Button block don't work correctly because they're being applied to the inner element, whereas they should be on the Button wrapper.
This PR ensures those styles are targeting the correct element.
Testing Instructions
Use of AI Tools
Code written with GPT 5.5/codex.